[v7r3] move Script goods to DIRACScript#5035
Conversation
|
@TaykYoku This looks like a really nice change to remove some of the global state. Can you add an explanation of what is being changed to the PR description to make this easier to review? Also given how large the changes are and how poorly tested the scripts are in the CI I think this might be better suited to v7r3. If others agree I can come up with a hack to workaround the problem mentioned in #5032 (comment). |
|
@chrisburr agree that many fixes delay the release. |
fstagni
left a comment
There was a problem hiding this comment.
There are some scripts also inside the /tests directory
|
given that this PR will go to v7r3 as well as #5024, then I can combine them here, to avoid duplication of work on their review. They both affect the same files |
f69c661 to
e4251ee
Compare
|
Perhaps it would be a good idea to discuss how you propose to change the scripts before doing the tedious work of converting them all? |
|
I wanted to protest whether it would work in all cases, but I think you are right. P.S.: now I'm not improving anything, I'm just fixing bugs to pass the tests(this will simplify reviewing) |
556a6ac to
4e6b308
Compare
fb68bd5 to
ce8a5cf
Compare
|
restart tests |
|
retest |
|
since the main issue was solved by a separate PR, I rebase/simplified this PR by leaving the transfer of goods from Script and registering positional arguments to display them in the help message. |
…nds/dirac_ping_info.py Co-authored-by: fstagni <federico.stagni@cern.ch>
3527eb9 to
6a130d2
Compare
…a_status.py Co-authored-by: fstagni <federico.stagni@cern.ch>
…request.py Co-authored-by: fstagni <federico.stagni@cern.ch>
Co-authored-by: fstagni <federico.stagni@cern.ch>
UPDATE:
additional testing did not reveal similar problems described here, so this reason I think is leveled
UPDATE:
DIRACScriptmade before merging were required to edit each affectedDIRACScriptscript, so I have added argument registration for "full testing".More about PR, since
DIRACScriptwill be used every time the command is called, the main change concerns this class, namely the transfer of logic fromScriptas instance variables/methods.Added the ability to register positional arguments and add it to help message from #5024.
Note: Applied
Main changes:
DIRACScript:ScriptDIRACScriptssearch cycle among theentry_points, a filter byscriptNameis added, @chrisburr could you review this change, please?UPDATE: This already done with #5061
Changes for the scripts(e.g.: dirac-my-great-script):
registerArgumentScriptimportto
BEGINRELEASENOTES
*allSystems/scripts
CHANGE: use registerArgument to register positional arguments for scripts
FIX: use DIRACScript instead Script
ENDRELEASENOTES